Repository navigation
fix(schemer): fail table migration when the peer schema cannot be read - #2144
Open
theluckystrike wants to merge 1 commit into
Open
theluckystrike wants to merge 1 commit into
theluckystrike wants to merge 1 commit into
Conversation
queryUnzipColumns returned nil when QueryAny failed on every peer, debugCreateSQLs dropped the error and createTablesSQLs ignored it, so HostCreateTables ran no DDL and returned nil. migrateTables then recorded the host in hostsWithTablesCreated and never migrated it again. Propagate the error up to HostCreateTables so migrateTables defers the host and retries the migration. Fixes Altinity#2129 Signed-off-by: theluckystrike <51033404+theluckystrike@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2129.
When the operator adds tables to a replica, it reads the CREATE statements from the other hosts of the cluster. If that read failed on every peer, the failure looked like an empty schema.
queryUnzipColumnsreturned nil whenQueryAnyfailed,debugCreateSQLslogged the error and dropped it, andcreateTablesSQLsignored the error of both getters. SoHostCreateTablesran no DDL and returned nil,migrateTableslogged "Tables added successfully" and put the host instatus.hostsWithTablesCreated. After thatshouldMigrateTablesskips the host, so the rebuilt replica stays without tables. The issue hits this when all peers answer Code 516 because the users.d ConfigMap with the new operator pod IP has not reached them yet.This change passes the error up instead.
queryUnzipColumnsreturns theQueryAnyerror,debugCreateSQLsreturns the error it was given, the replicated and distributed getters stop at the first failed query, andHostCreateTablesreturns "unable to read schema from peers of host ..." without running any DDL.migrateTablesalready wraps aHostCreateTableserror inErrCRUDDeferredand does not record the host, so the migration runs again on a later reconcile.Behaviour stays the same in the other cases.
QueryAnystill uses the first peer that answers, so the error only appears when no peer could be read. A done context still returns nil, as before. New installations skip the migration throughIsInNewCluster.HostSyncTablesand the drop replica path callQueryUnzip2Columnsand already ignore its error, so they behave as before.Two tests were added in
pkg/model/chi/schemer/schemer_test.go.TestQueryUnzipReportsRejectedPeerpointsQueryUnzip2ColumnsandQueryUnzipAndApplyUUIDsat a local httptest server that answers HTTP 403 with a Code 516 body and expects an error.TestHostCreateTablesFailsWhenPeersCannotBeReadbuilds a CHI with 1 shard and 2 replicas whosenamespaceDomainPatternends in.invalid, so the peer cannot be reached, and expectsHostCreateTablesto return an error. On 0.28.0 (5d6c2fe) both fail with "An error is expected but got nil". With this change both pass.I ran
go test -vet=off -count=1 ./...on 0.28.0 and on this branch with Go 1.27.3 on macOS. This branch has 40 packages ok and 2337 passing tests, and 0.28.0 has 39 and 2335. The difference is the two new tests in a package that had no tests before. Both runs show the same 325 failing test lines, all inpkg/model/chbackup, because the job scripts need bash 4.4 and macOS ships bash 3.2. I used-vet=offbecause with Go 1.27.3 the vet step ofgo teststopspkg/controller/chi,pkg/controller/chkandpkg/controller/common/statefulseton non-constant format strings in files this PR does not touch. The same happens on 0.28.0. With vet off those packages pass.go vet ./pkg/model/chi/schemer/ ./pkg/model/clickhouse/is clean and gofmt reports nothing for the changed files.Not run. The e2e suite, a real ClickHouse server and a Kubernetes cluster. I did not reproduce the PVC deletion from the issue on a live cluster.
Template checklist. The PR is one commit, it targets 0.28.0 and the commit is signed off.